Move add_builtin to SVM#547
Conversation
|
@pgarg66 mentioned he wanted to have a look at these changes, so I'll wait for his review before merging. |
| } | ||
|
|
||
| fn add_builtin_account(&self, _name: &str, _program_id: &Pubkey, _must_replace: bool) { | ||
| todo!() |
There was a problem hiding this comment.
There is still a todo!() left over here.
There was a problem hiding this comment.
@LucasSte if this is an optional trait method, maybe have a default implementation of it in the trait definition?
There was a problem hiding this comment.
It is an optional method, but coming up with a default implementation is not straightforward. In order to add a builtin account, we need access to a generic accounts database, which developers will only provide when they create their own struct to replace Agave's Bank.
There was a problem hiding this comment.
The default implementation can be empty. So you don't need to implement for test with a todo in it. If the users of SVM do not need builtin they can ignore it.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #547 +/- ##
=========================================
- Coverage 81.9% 81.9% -0.1%
=========================================
Files 851 851
Lines 230475 230485 +10
=========================================
- Hits 188785 188774 -11
- Misses 41690 41711 +21 |
`slow-rebase.sh` was actually printing incorrect commands - copy/pasting them failed. This is an attempt to compute correct commands from the standpoint of the user running the script.
5 participants
Problem
Presently, external SVM users must register builtins in two different places separately: the program cache and a vector passed to
load_and_process_sanitized_transactions. These steps are burdensome and can lead to potential mistakes. In addition, the execution of programs should be self contained in the SVM, so the registration of builtins is semantically related to the SVM scope.Summary of Changes
add_builtinfrombank.rsto the SVM folder.add_builtin_accountis now part of the traitTransactionProcessingCallback.builtin_program_idsfromstruct Banktostruct TransactionBatchProcessor.Fixes #303